You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Improve state safety, keep MCP activity visible, and make bulk file operations more reliable without including the RPM packaging changes from #201. This branch is based directly on main.
Serialize shell and MCP command execution, including nested shell calls, and preserve the existing cancellation behavior.
Include the connected account and navigation context in destructive MCP confirmations. Refuse execution when connection or navigation changes while awaiting approval, including navigating away and back.
Keep echoing MCP command lines and recording them in history so their activity stays visible in the shell. Echoing and the line editor no longer fail the command they announce when the host has no ANSI terminal. History remains complete and replayable, including connection strings; document its sensitivity.
Write exports through a temporary file in the destination directory and replace the destination only after successful completion. Preserve existing files on failures and cancellation, respect overwrite protection, dispose query iterators, and stop requesting pages once --max is reached.
Read CSV imports incrementally instead of loading the whole file into memory, and parse them with CsvHelper 33.1.0 so a malformed record aborts with its physical line number instead of silently absorbing the rest of the file. Files that previously imported without an error can now be rejected. Spool CSV export documents to private temporary storage rather than retaining all documents in memory, preserving the complete dynamic column set. Flush JSON array output incrementally.
Add regression coverage and update README, command reference, MCP security documentation, history documentation, and localized CSV errors.
Operational Notes
MCP clients still share one connection and navigation context; explicit database/container arguments remain recommended. Confirmation waits do not hold the execution lock.
CSV export requires temporary disk space for its JSON spool in addition to the destination export.
Imports are not transactional: earlier writes remain if a later record fails. Use --dry-run to validate the entire input first.
Abrupt process termination can leave an unfinished destination-directory export temporary file.
Serialize shell and MCP execution and invalidate destructive confirmations after context changes. Preserve replayable interactive history while omitting MCP command echo and history entries.
Write exports through temporary files, stop pagination at the requested limit, dispose iterators, and spool CSV exports to disk. Stream CSV imports with CsvHelper and report malformed record line numbers.
Add regression tests and document history policy, context sharing, and export guarantees. Offline validation: 2315 passed, 2 interactive-console tests skipped.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
It introduces a serialized execution semaphore that is not disposed and an export temp-file cleanup path that can mask the original failure, both of which should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens MCP tool execution against shared-shell state changes and improves import/export reliability and scalability, while updating user-facing documentation and adding regression tests across MCP, serialization, and bulk file operations.
Changes:
Serialize shell + MCP command execution and invalidate destructive MCP confirmations when connection/navigation context changes.
Make export atomic via destination-directory temp files, improve iterator/page-limit handling, and spool CSV exports to disk; stream CSV imports with CsvHelper and better error localization.
Update README/docs and add regression tests covering the new behavior.
File summaries
File
Description
README.md
Documents atomic export behavior, serialized MCP execution, and history sensitivity.
docs/navigation.md
Documents sensitive history contents and MCP history/echo behavior.
docs/mcp.md
Documents destructive confirmation context + invalidation and serialized execution model.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
A couple of small but concrete issues remain (notably best-effort temp-file cleanup not covering all failure modes, and a potentially misleading trace log message) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
This trace log says "Invoking" even though the command may be refused later if the confirmation context changed; that can make diagnostics misleading. Consider logging this as a request rather than an invocation (or move the log to just before ExecuteCosmosCommandAsync after the version check).
Second review follow-up in 2e2a58d: the suppressed trace finding is fixed by logging request handling instead of claiming execution before context validation. The DirectoryNotFoundException thread was already covered by IOException handling; an explicit regression now verifies it. Validation: 59 focused export/MCP tests passed, both projects built without warnings. All current review threads have explanatory replies and are resolved.
MCP invocations are currently appended only to the in-memory history by PrintCommand; SaveHistory is called only after a later interactive command in RunAsync. An MCP-only session can therefore exit without writing any of its commands to cmd_history, contrary to this statement that they are recorded in the same replayable history. Persist the history when recording an MCP invocation and add a restart/persistence regression test.
This records MCP commands only in memory. SaveHistory() is called solely from the interactive input path, so an MCP invocation is absent from cmd_history unless a later interactive command happens to trigger a save; an MCP-only session also never applies MAXHISTORYITEMS. Persist MCP entries through a synchronized history update so the documented history remains available after restart and stays bounded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improve state safety, keep MCP activity visible, and make bulk file operations more reliable without including the RPM packaging changes from #201. This branch is based directly on
main.--maxis reached.Operational Notes
--dry-runto validate the entire input first.